DAOS-14535 pool: start/stop ds_pool_child individually #13347

NiuYawei · 2023-11-16T03:33:35Z

Reorganize ds_pool_child start/stop code to make it able to be
started/stopped individually:

Introduced four states for ds_pool_child: NEW, STARTING, STARTED
and STOPPING.
ds_pool_child is added/removed to/from cache through the collective
call of pool_child_add/delete_one(), now the cache doesn't hold the
ds_pool_child reference anymore.
Introduced ds_pool_child_start/stop() for callers to start/stop
ds_pool_child individually, ds_pool_child_state() to query the state
of ds_pool_child.
Adjusted ds_pool_child_lookup() & ds_pool_child_get() a bit;

Required-githooks: true

Signed-off-by: Niu Yawei yawei.niu@intel.com

Before requesting gatekeeper:

Two review approvals and any prior change requests have been resolved.
Testing is complete and all tests passed or there is a reason documented in the PR why it should be force landed and forced-landing tag is set.
Features: (or Test-tag*) commit pragma was used or there is a reason documented that there are no appropriate tags for this PR.
Commit messages follows the guidelines outlined here.
Any tests skipped by the ticket being addressed have been run and passed in the PR.

Gatekeeper:

github-actions · 2023-11-16T03:33:52Z

Bug-tracker data:
Ticket title is 'Handle WAL/Meta SSD faulty'
Status is 'In Progress'
Labels: 'md_on_ssd'
https://daosio.atlassian.net/browse/DAOS-14535

daosbuild1

LGTM. No errors found by checkpatch.

daosbuild1 · 2023-11-16T03:59:30Z

Test stage Build on Leap 15.4 with Intel-C and TARGET_PREFIX completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-13347/1/execution/node/582/log

daosbuild1

LGTM. No errors found by checkpatch.

daosbuild1 · 2023-11-16T04:43:58Z

Test stage Build on Leap 15.4 with Intel-C and TARGET_PREFIX completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-13347/2/execution/node/390/log

daosbuild1 · 2023-11-16T04:48:17Z

Test stage Build RPM on EL 9 completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-13347/2/execution/node/313/log

daosbuild1 · 2023-11-16T04:51:04Z

Test stage Build RPM on EL 8 completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-13347/2/execution/node/393/log

daosbuild1 · 2023-11-16T04:52:52Z

Test stage Build RPM on Leap 15.4 completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-13347/2/execution/node/325/log

daosbuild1 · 2023-11-16T04:54:40Z

Test stage Build DEB on Ubuntu 20.04 completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-13347/2/execution/node/355/log

daosbuild1

LGTM. No errors found by checkpatch.

daosbuild1 · 2023-11-16T13:36:57Z

Test stage Functional on EL 8 completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-13347/3/execution/node/1130/log

Reorganize ds_pool_child start/stop code to make it able to be started/stopped individually: - Introduced four states for ds_pool_child: NEW, STARTING, STARTED and STOPPING. - ds_pool_child is added/removed to/from cache through the collective call of pool_child_add/delete_one(), now the cache doesn't hold the ds_pool_child reference anymore. - Introduced ds_pool_child_start/stop() for callers to start/stop ds_pool_child individually, ds_pool_child_state() to query the state of ds_pool_child. - Adjusted ds_pool_child_lookup() & ds_pool_child_get() a bit; Required-githooks: true Signed-off-by: Niu Yawei <yawei.niu@intel.com>

daosbuild1

LGTM. No errors found by checkpatch.

liw · 2023-11-21T00:51:01Z

src/pool/srv_util.c

@@ -973,7 +973,6 @@ check_pool_targets(uuid_t pool_id, int *tgt_ids, int tgt_cnt, bool reint,
 	}

 	ABT_rwlock_unlock(pool->sp_lock);
-	ds_pool_child_put(pool_child);


The ABT_rwlock_rdlock call on line 937 may cause the current ULT to yield. So, it's not easy to why not holding a pool_child reference is safe.

indeed, it's better to call ds_pool_child_get() to hold reference. Can I fix it in the follow-on PR (along with the two spelling issues.)?

Fine with me.

liw · 2023-11-21T02:21:35Z

src/pool/srv_target.c

 			return child;
 		}
 	}
 	return NULL;
 }

 struct ds_pool_child *
-ds_pool_child_get(struct ds_pool_child *child)
+ds_pool_child_get(const uuid_t uuid)


How about adding a static pool_child_lookup_noref (or ds_pool_child_lookup_noref if you need an external one) and keeping the semantics of ds_pool_child_lookup unchanged (except for checking the state)? And we could remove ds_pool_child_get if necessary.

That seems to have less impact on other modules, and also more consistent with other "lookup/get/put" APIs we have.

Ok, sounds good to me. I'll fix it.

Address review comments from LiWei. Fix spelling errors. Required-githooks: true Signed-off-by: Niu Yawei <yawei.niu@intel.com>

daosbuild1

LGTM. No errors found by checkpatch.

liuxuezhao · 2023-11-22T03:01:30Z

src/pool/srv_target.c

-		dss_module_fini_metrics(DAOS_TGT_TAG, child->spc_metrics);
-		ABT_eventual_set(child->spc_ref_eventual,
-				 (void *)&child->spc_ref,
+	if (child->spc_ref == 0 && *child->spc_state == POOL_CHILD_STOPPING)


seems when the spc_ref drop to zero, the spc_state must be STOPPING?

pool child cache doesn't hold pool child reference now, so the pool child reference could be dropped to zero even if it's still in cache (in start mode).

) Reorganize ds_pool_child start/stop code to make it able to be started/stopped individually: - Introduced four states for ds_pool_child: NEW, STARTING, STARTED and STOPPING. - ds_pool_child is added/removed to/from cache through the collective call of pool_child_add/delete_one(), now the cache doesn't hold the ds_pool_child reference anymore. - Introduced ds_pool_child_start/stop() for callers to start/stop ds_pool_child individually, ds_pool_child_state() to query the state of ds_pool_child. - Removed ds_pool_child_get(); Signed-off-by: Niu Yawei <yawei.niu@intel.com> Signed-off-by: shiying <yshi1210@gmail.com>

Reorganize ds_pool_child start/stop code to make it able to be started/stopped individually: - Introduced four states for ds_pool_child: NEW, STARTING, STARTED and STOPPING. - ds_pool_child is added/removed to/from cache through the collective call of pool_child_add/delete_one(), now the cache doesn't hold the ds_pool_child reference anymore. - Introduced ds_pool_child_start/stop() for callers to start/stop ds_pool_child individually, ds_pool_child_state() to query the state of ds_pool_child. - Removed ds_pool_child_get(); Signed-off-by: Niu Yawei <yawei.niu@intel.com>

daosbuild1 reviewed Nov 16, 2023

View reviewed changes

NiuYawei force-pushed the niu/DAOS-14535-pool-child branch from da2ee07 to afec482 Compare November 16, 2023 04:37

daosbuild1 reviewed Nov 16, 2023

View reviewed changes

NiuYawei force-pushed the niu/DAOS-14535-pool-child branch from afec482 to 601c8cb Compare November 16, 2023 09:24

daosbuild1 reviewed Nov 16, 2023

View reviewed changes

NiuYawei force-pushed the niu/DAOS-14535-pool-child branch from 601c8cb to 8bfc948 Compare November 17, 2023 02:09

daosbuild1 reviewed Nov 17, 2023

View reviewed changes

NiuYawei requested review from liuxuezhao, liw, Nasf-Fan and wangshilong November 20, 2023 01:28

liw reviewed Nov 21, 2023

View reviewed changes

wangshilong previously approved these changes Nov 21, 2023

View reviewed changes

DAOS-14535 pool: address review comments

bb4c085

Address review comments from LiWei. Fix spelling errors. Required-githooks: true Signed-off-by: Niu Yawei <yawei.niu@intel.com>

NiuYawei dismissed wangshilong’s stale review via bb4c085 November 21, 2023 06:54

daosbuild1 reviewed Nov 21, 2023

View reviewed changes

NiuYawei requested review from liw and wangshilong November 21, 2023 06:59

liw approved these changes Nov 21, 2023

View reviewed changes

liuxuezhao approved these changes Nov 22, 2023

View reviewed changes

NiuYawei merged commit 3bebffe into master Nov 22, 2023
44 of 45 checks passed

NiuYawei deleted the niu/DAOS-14535-pool-child branch November 22, 2023 06:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DAOS-14535 pool: start/stop ds_pool_child individually #13347

DAOS-14535 pool: start/stop ds_pool_child individually #13347

NiuYawei commented Nov 16, 2023 •

edited

Loading

github-actions bot commented Nov 16, 2023

daosbuild1 left a comment

daosbuild1 commented Nov 16, 2023

daosbuild1 left a comment

daosbuild1 commented Nov 16, 2023

daosbuild1 commented Nov 16, 2023

daosbuild1 commented Nov 16, 2023

daosbuild1 commented Nov 16, 2023

daosbuild1 commented Nov 16, 2023

daosbuild1 left a comment

daosbuild1 commented Nov 16, 2023

daosbuild1 left a comment

liw Nov 21, 2023

NiuYawei Nov 21, 2023

liw Nov 21, 2023

liw Nov 21, 2023

NiuYawei Nov 21, 2023

daosbuild1 left a comment

liuxuezhao Nov 22, 2023

NiuYawei Nov 22, 2023

DAOS-14535 pool: start/stop ds_pool_child individually #13347

DAOS-14535 pool: start/stop ds_pool_child individually #13347

Conversation

NiuYawei commented Nov 16, 2023 • edited Loading

Before requesting gatekeeper:

Gatekeeper:

github-actions bot commented Nov 16, 2023

daosbuild1 left a comment

Choose a reason for hiding this comment

daosbuild1 commented Nov 16, 2023

daosbuild1 left a comment

Choose a reason for hiding this comment

daosbuild1 commented Nov 16, 2023

daosbuild1 commented Nov 16, 2023

daosbuild1 commented Nov 16, 2023

daosbuild1 commented Nov 16, 2023

daosbuild1 commented Nov 16, 2023

daosbuild1 left a comment

Choose a reason for hiding this comment

daosbuild1 commented Nov 16, 2023

daosbuild1 left a comment

Choose a reason for hiding this comment

liw Nov 21, 2023

Choose a reason for hiding this comment

NiuYawei Nov 21, 2023

Choose a reason for hiding this comment

liw Nov 21, 2023

Choose a reason for hiding this comment

liw Nov 21, 2023

Choose a reason for hiding this comment

NiuYawei Nov 21, 2023

Choose a reason for hiding this comment

daosbuild1 left a comment

Choose a reason for hiding this comment

liuxuezhao Nov 22, 2023

Choose a reason for hiding this comment

NiuYawei Nov 22, 2023

Choose a reason for hiding this comment

NiuYawei commented Nov 16, 2023 •

edited

Loading